Fix create data structure from ome.tif#1104
Merged
Merged
Conversation
Member
|
Hi, I didnt check the functionality but I assume you tried if it worked and also if it breaks old implementation. I will approve, but wait until copilot is done and see if it finds anything |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes issues encountered when creating a data structure from .ome.tif files by (1) relaxing filename validation to permit the .ome.tif suffix and (2) handling cases where OME-XML metadata is missing in TIFF tags.
Changes:
- Allow
.ome.tiffilenames indataStruct.checkFileNames()by passing anallowedsuffix to filename validation. - Extend
acdc_regex.is_alphanumeric_filename()with anallowedsuffix parameter to support multi-dot extensions. - Guard OME-XML parsing in
load.OMEXML.parse_metadata()when no OME metadata is present.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cellacdc/load.py | Avoids parsing OME-XML when tif.ome_metadata is missing. |
| cellacdc/dataStruct.py | Permits .ome.tif filenames during raw data structure creation. |
| cellacdc/acdc_regex.py | Adds support for allowed multi-dot suffixes in filename validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+43
to
+44
| if allowed is not None: | ||
| max_num_dots += sum([txt.count('.') for txt in allowed]) |
Comment on lines
+46
to
+48
| for allowed_text in allowed: | ||
| allowed_text = re.escape(allowed_text) | ||
| pattern = pattern.replace(r'+$', fr'+({allowed_text})?$') |
Comment on lines
+26
to
+30
| def is_alphanumeric_filename( | ||
| text, | ||
| allow_space=True, | ||
| allowed: str | list[str] | None=None | ||
| ): |
Comment on lines
+3733
to
3738
| if self.root is None: | ||
| return 'undefined' | ||
|
|
||
| Image = self.root.find(f'{self.ome_schema}Image') | ||
| Pixels = Image.find(f'{self.ome_schema}Pixels') | ||
| image = OMEXML_image(Pixels, self.ome_schema) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements a fix for creating the data structure from
.ome.tiffiles.More specifically:
'.ome.tif'must be allowed in the filename at lineCell_ACDC/cellacdc/dataStruct.py
Line 1931 in f1843a9
allowed--> `allowed = ( '.ome.tif',)self.omexml_stringat lineCell_ACDC/cellacdc/load.py
Line 3700 in f1843a9